Skip to content

perf(issue): skip redundant API lookups via project+issue-org caches#794

Merged
BYK merged 3 commits intomainfrom
fix/cache-project-and-issue-org
Apr 21, 2026
Merged

perf(issue): skip redundant API lookups via project+issue-org caches#794
BYK merged 3 commits intomainfrom
fix/cache-project-and-issue-org

Conversation

@BYK
Copy link
Copy Markdown
Member

@BYK BYK commented Apr 21, 2026

Summary

Addresses two of the Consecutive HTTP performance issues reported in Sentry for the cli project by caching slug→ID and id→org mappings so repeat invocations skip redundant sequential API calls:

  • Pattern C (CLI-1BK)sentry issue list <org>/<project> used to fire a preflight GET /projects/{org}/{project}/ before every GET /issues/. Now skipped when the project is already in project_cache (populated by listProjects(), DSN resolution, and — new in this PR — by fetchProjectId itself).
  • Pattern D (partial, related to CLI-19S / CLI-19W)sentry issue view <numeric-id> without org context used to fall back to the legacy unscoped /issues/{id}/ endpoint on every run. A new issue_org_cache SQLite table (schema v15) maps numeric-issue-id → org-slug so repeat runs go straight to the regional org-scoped endpoint.

Other Consecutive HTTP issues reported against this project are strict data dependencies (e.g. trace_id lives inside the event body, so /trace/ cannot fire before /events/latest/) or cursor-based pagination the API cannot serve in parallel. Those were triaged and archived in Sentry.

Changes

Pattern C — fetchProjectId cache-first

  • src/lib/db/project-cache.ts — new getCachedProjectBySlug(orgSlug, projectSlug) that looks up the most-recent entry across all three existing project_cache key shapes (orgId:projectId, dsn:publicKey, list:{org}/{project}) via MAX(cached_at).
  • src/lib/resolve-target.tsfetchProjectId now:
    1. Consults the cache before calling GET /api/0/projects/{org}/{project}/.
    2. On API success, writes the response back to the cache so the next call skips the round trip. Guarded with try/catch so a broken/read-only DB does not crash the successful API path.

Pattern D — numeric-issue-id → org cache

  • src/lib/db/schema.ts — new issue_org_cache SQLite table with schema v14 → v15 migration (issue_id TEXT PK, org_slug TEXT, cached_at INTEGER).
  • src/lib/db/issue-org-cache.ts — new module with getCachedIssueOrg, setCachedIssueOrg, clearCachedIssueOrg, clearAllIssueOrgCache. No TTL — issues are owned by one org for life; stale-mapping detection handled via 404 eviction.
  • src/commands/issue/utils.ts:
    • Extracted helper fetchIssueByNumericId(id, explicitOrg, cachedOrg) that returns { issue, cacheEvicted }.
    • resolveNumericIssue and resolveShareIssue now:
      1. Prefer explicit org context (resolveOrg — env vars, config defaults, DSN).
      2. Try the cached org next, routing via the regional org-scoped endpoint.
      3. Fall back to the legacy unscoped endpoint otherwise.
      4. On 404 from the cached-org call: evict the entry, retry unscoped, and set cacheEvicted = true so the caller discards the stale slug and re-derives from the permalink (re-caching the correct value). 5xx errors propagate without eviction — transient failures must not forget valid mappings.
      5. After a cold-path permalink extraction, record the numeric-id → org mapping. Cache writes are try/catch-guarded so a broken DB never crashes the successful API path.
  • src/lib/db/auth.tsclearAuth() calls clearAllIssueOrgCache() on logout so mappings don't leak across accounts.

The stale-cache bug (Seer + Cursor Bugbot caught this)

The initial implementation had a subtle bug: when fetchIssueByNumericId internally caught a 404 from the cached-org endpoint, evicted the entry, and fell back to the unscoped endpoint, the caller's local cachedOrg variable still held the stale slug and won the ?? chain over extractOrgFromPermalink(issue.permalink). Downstream calls (events, traces) would then target the wrong org.

Fix: the helper returns { issue, cacheEvicted }; callers gate cachedOrg through effectiveCachedOrg = cacheEvicted ? null : cachedOrg, and re-cache the corrected mapping from the permalink. Regression test in test/commands/issue/utils.test.ts explicitly asserts the returned org is the correct one, not the stale one.

Tests

  • test/lib/db/project-cache.test.ts — new tests for getCachedProjectBySlug across all three cache key shapes, MAX-by-cached_at tiebreak, org/project mismatch, and legacy rows with null project_id. Merged into the existing clearProjectCache suite with added getCachedProjectBySlug assertions.
  • test/lib/db/issue-org-cache.test.ts — new file with 11 tests covering get/set/clear/clear-all semantics, empty-key no-ops, and isolation from unrelated tables.
  • test/lib/resolve-target.test.ts — new tests for fetchProjectId covering cache-hit (no API call), write-back on miss, and fall-through when a cached row has no projectId (legacy schema rows).
  • test/commands/issue/utils.test.ts — new describe("Pattern D") block with 4 tests:
    • Warm cache → single org-scoped call, no legacy fallback.
    • Stale-cache fix: 404 on cached org → helper evicts, falls back, and returns the correct permalink-derived org (NOT the stale slug).
    • Cold path → legacy fallback populates the cache for next time.
    • 5xx on cached-org call propagates without evicting the cache (transient failure protection).
  • test/lib/db/auth.test.ts — new integration test asserting clearAuth() drops issue_org_cache entries.

All tests green locally: 5106 unit + 138 isolated + 171 on impacted files. Typecheck clean, lint clean (only pre-existing unrelated warning in markdown.ts).

Archived Sentry issues (non-actionable Consecutive HTTP)

For reference, these were marked archived_forever as part of this triage:

Why not leverage the existing HTTP response cache for the issue-id → org mapping?

(Short answer: different lookup direction + TTL mismatch.)

The HTTP response cache (createAuthenticatedFetch + http-cache-semantics) keys by request URL. It answers "what was the body of GET /issues/123/ last time?", not "which org owns issue 123?" — the org is derived from the response body's permalink, not from any URL the cache could index. Even if we could answer that, the HTTP cache honors server TTLs (60s–5min for Sentry's typical GET responses), whereas the issue→org mapping is effectively permanent for an issue's lifetime. And crucially: relying on the HTTP cache still requires firing the unscoped endpoint at least once per session, which is the exact call the issue_org_cache lookup skips. The two caches compose orthogonally — HTTP cache speeds up repeat identical requests; issue_org_cache eliminates the need for the unscoped request at all on warm runs.

…lookups

Addresses two 'Consecutive HTTP' performance issues reported in Sentry where
the CLI fires avoidable sequential API calls on hot paths:

- **Pattern C (CLI-1BK)** — `sentry issue list <org>/<project>` fires a
  preflight `GET /projects/{org}/{project}/` before every `GET /issues/`.
  `fetchProjectId` in src/lib/resolve-target.ts now consults project_cache
  via a new `getCachedProjectBySlug(org, project)` helper before calling
  the API, and seeds the cache on API success. `listProjects()` and DSN
  resolution already populate the cache — `fetchProjectId` was the one
  hot-path caller that bypassed it.

- **Pattern D (CLI-19S/CLI-19W partial)** — `sentry issue view <numeric-id>`
  without an org context falls back to the legacy unscoped endpoint, then
  extracts the org from the response permalink. Subsequent runs repeat the
  same fallback. A new `src/lib/db/issue-org-cache.ts` module stores the
  numeric-id → org-slug mapping in the existing `metadata` table (no schema
  migration); `resolveNumericIssue` and `resolveShareIssue` now use it to
  go straight to the org-scoped endpoint on repeat runs. Stale entries are
  evicted on 404 so the legacy fallback still covers moved/deleted issues.
  The cache is cleared on `auth logout` to avoid cross-account leakage.

Also adds `clearMetadataByPrefix` to src/lib/db/utils.ts so both auth.ts
and issue-org-cache.ts satisfy the project's 'no raw metadata queries' lint
rule. Extracts `fetchIssueByNumericId` from `resolveNumericIssue` to keep
cognitive complexity under the 15-point cap.

The other Consecutive HTTP patterns (issue→event→trace chains and
cursor-based pagination) are true data dependencies and not addressable
here — they have been archived in Sentry.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 21, 2026

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

Bug Fixes 🐛

Internal Changes 🔧

  • (init) Trim deprecated --features help entries by MathurAditya724 in #781
  • (issue) Skip redundant API lookups via project+issue-org caches by BYK in #794
  • Regenerate docs by github-actions[bot] in 58a84035

🤖 This preview updates automatically when you update the PR.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 21, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://cli.sentry.dev/_preview/pr-794/

Built to branch gh-pages at 2026-04-21 11:14 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 21, 2026

Codecov Results 📊

138 passed | Total: 138 | Pass Rate: 100% | Execution Time: 0ms

📊 Comparison with Base Branch

Metric Change
Total Tests
Passed Tests
Failed Tests
Skipped Tests

✨ No test changes detected

All tests are passing successfully.

✅ Patch coverage is 97.40%. Project has 1708 uncovered lines.
✅ Project coverage is 95.59%. Comparing base (base) to head (head).

Files with missing lines (1)
File Patch % Lines
src/lib/resolve-target.ts 82.61% ⚠️ 4 Missing
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    95.57%    95.59%    +0.02%
==========================================
  Files          265       266        +1
  Lines        38581     38719      +138
  Branches         0         0         —
==========================================
+ Hits         36872     37011      +139
- Misses        1709      1708        -1
- Partials         0         0         —

Generated by Codecov Action

Comment thread src/commands/issue/utils.ts
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a3a027b. Configure here.

Comment thread src/commands/issue/utils.ts Outdated
Comment thread src/lib/db/issue-org-cache.ts
@BYK BYK changed the title fix(perf): cache project slug and numeric-issue-id to skip redundant lookups perf: cache project slug and numeric-issue-id to skip redundant lookups Apr 21, 2026
Comment thread src/lib/db/auth.ts Outdated
BYK added 2 commits April 21, 2026 10:45
Address review feedback:

- **BYK**: migrate issue-org cache from the `metadata` KV table to a
  dedicated `issue_org_cache` table (schema v15 with migration). The
  metadata KV shoehorning was called out as poor practice; dedicated
  table is cleaner and matches the pattern of other cache tables.

- **Sentry Seer + Cursor Bugbot**: fix stale-cachedOrg leak after 404
  fallback. When `fetchIssueByNumericId` internally catches a 404 from
  the cached org, evicts the cache entry, and falls back to the legacy
  unscoped endpoint, the caller's local `cachedOrg` still held the
  stale slug and won the `??` chain over `extractOrgFromPermalink()`.
  Solution: helper now returns `{ issue, cacheEvicted }`; callers treat
  their local `cachedOrg` as null when `cacheEvicted` is true and
  re-derive from the permalink. Also re-writes the corrected mapping
  to the cache so the next run uses the right org. Applies to both
  `resolveNumericIssue` and `resolveShareIssue`.

- **Cursor Bugbot**: use `clearAllIssueOrgCache()` from auth.ts instead
  of duplicating the table-clear logic with a hardcoded prefix. The
  `clearMetadataByPrefix` helper is no longer needed and has been
  removed — the dedicated table handles its own clearing.

Tests:
- 3 new tests in test/commands/issue/utils.test.ts cover the warm-cache,
  stale-cache-eviction, and permalink-extraction-then-cache paths with
  explicit assertions that stale slugs do NOT leak to downstream calls.
Self-review with a critical-eye subagent surfaced one major and several
minor issues that were worth fixing before merge:

- **M1 (major)**: Three new cache writes (`setCachedProject` in
  `fetchProjectId`, two `setCachedIssueOrg` call sites in
  `resolveNumericIssue` / `resolveShareIssue`) ran without try/catch,
  violating the AGENTS.md invariant that non-essential DB cache writes
  must not crash a command whose primary API call already succeeded.
  Each write is now guarded and logs a debug message on failure,
  matching the `setInstallInfo`/`setUserInfo` pattern in login.ts
  and upgrade.ts.

- **m2 (minor)**: Added a test verifying that a 5xx from the cached-org
  endpoint propagates the error WITHOUT evicting the cache entry (only
  404 means 'stale'). Covers the invariant that transient failures
  shouldn't cause us to forget correct mappings.

- **n1**: Skip `cachedOrg` in the error-hint fallback for 404s. When
  both the cached-org call AND the legacy unscoped call 404, the cache
  entry has already been evicted as stale — suggesting that stale org
  in 'Try: sentry issue view <stale-org>/<id>' would mislead the user.
  Use the generic `<org>` placeholder instead.

- **n3**: Drop the bogus `& { 'MAX(cached_at)'?: number }` type
  extension in `getCachedProjectBySlug` — the SQL `AS cached_at`
  alias means the result row uses `cached_at`, not `MAX(cached_at)`.

- **n4**: Added `clearAuth` integration test that verifies issue-org
  mappings are dropped on logout (prevents cross-account leakage).

- **n5**: Fixed duplicate/misplaced JSDoc on `fetchIssueByNumericId` —
  type alias and function declaration each get their own docblock.

- **n6**: Consolidated two `describe('clearProjectCache', ...)` blocks
  into one by adding `getCachedProjectBySlug` assertions to the
  existing test; removed the duplicate suite.

- **n2** (trivial): Updated a test description that still used the old
  metadata-prefix wording ('removes all issue_org.* entries' →
  'removes all cached mappings').
@BYK BYK changed the title perf: cache project slug and numeric-issue-id to skip redundant lookups perf(issue): skip redundant API lookups via project+issue-org caches Apr 21, 2026
@BYK BYK merged commit 405928b into main Apr 21, 2026
27 checks passed
@BYK BYK deleted the fix/cache-project-and-issue-org branch April 21, 2026 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant